Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new secret primitive to the operator component framework, mirroring the existing static primitives (e.g., configmap) with builder/resource APIs, mutation + flavor pipelines, hashing utilities, tests, docs, and an end-to-end example.
Changes:
- Introduces
pkg/primitives/secret(resource, builder, mutator, flavors, and hash utilities) with comprehensive unit tests. - Adds a new shared editor
SecretDataEditorunderpkg/mutation/editorsto support typed.data/.stringDatamutations. - Adds documentation for the secret primitive plus a runnable
examples/secret-primitiveshowcasing composition via feature-gated mutations and flavors.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/primitives/secret/resource.go | Implements Secret Resource wrapper around internal generic static resource. |
| pkg/primitives/secret/resource_test.go | Tests identity/object deep-copy/mutation/data extraction behavior. |
| pkg/primitives/secret/mutator.go | Implements plan-and-apply mutator API for Secret metadata and data edits. |
| pkg/primitives/secret/mutator_test.go | Tests mutator editing helpers, ordering, and interface implementation. |
| pkg/primitives/secret/hash.go | Adds DataHash and Resource.DesiredHash utilities. |
| pkg/primitives/secret/hash_test.go | Tests determinism/sensitivity and DesiredHash behavior. |
| pkg/primitives/secret/flavors.go | Adds Secret-specific field application flavors incl. preserving external .data entries. |
| pkg/primitives/secret/flavors_test.go | Tests flavors and builder integration with Mutate. |
| pkg/primitives/secret/builder.go | Adds fluent builder API over internal generic static builder. |
| pkg/primitives/secret/builder_test.go | Tests builder validation and registration behavior. |
| pkg/mutation/editors/secretdata.go | Adds a shared typed editor for Secret .data and .stringData. |
| pkg/mutation/editors/secretdata_test.go | Tests SecretDataEditor behavior and nil-handling. |
| examples/secret-primitive/resources/secret.go | Example resource factory wiring mutations/flavors/extractors. |
| examples/secret-primitive/README.md | Example documentation and run instructions. |
| examples/secret-primitive/main.go | Runnable example using fake client + multiple reconciliation steps. |
| examples/secret-primitive/features/mutations.go | Example feature-gated Secret mutations using SetStringData + metadata edits. |
| examples/secret-primitive/features/flavors.go | Example flavor wrapper for preserving external entries. |
| examples/secret-primitive/app/controller.go | Example controller wiring component + Secret resource. |
| docs/primitives/secret.md | New user-facing documentation for the secret primitive (builder, ordering, flavors, hashing). |
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored:
|
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored: <!-- claude-review-cycle --> |
|
approved |
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored: <!-- claude-review-cycle --> |
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored:
<!-- claude-review-cycle --> |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…, and hash Implements the Secret primitive following the same pattern as the ConfigMap primitive. Includes full test coverage for builder validation, mutator operations, field application flavors, and data hashing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Demonstrates building a Secret resource with base credentials, version labels, and feature-gated tracing/metrics tokens using the secret primitive. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nd example - hash.go: Merge .stringData into a copy of .data before hashing to match Kubernetes API-server write semantics. Ensures DesiredHash reflects content set via SetStringData. - flavors.go: PreserveExternalEntries now treats keys present in applied.StringData as owned, preventing incorrect preservation of cluster values that the operator intends to overwrite via .stringData. - secret.md: Add nil-check for current.Data in the custom field applicator example to prevent panic. Update DataHash documentation to describe the merged hash semantics. - example secret.go: Remove misleading StringData iteration from the data extractor since .stringData is write-only and never returned by the API server on read. - Add tests for StringData-aware hash and flavor behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Print only key names and value lengths instead of base64-encoded values to prevent credential leakage if the example is copied into production. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Secret to the Built-in Primitives table and SecretDataEditor to the Mutation Editors table in docs/primitives.md so the new primitive is discoverable from the main index page. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aligns with the fix applied to deployment and configmap mutators in #42. The constructor now initializes the plans slice directly instead of calling beginFeature(), preventing an empty feature plan when mutate_helper.go calls fm.beginFeature() before each mutation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
docs/primitives.md
Outdated
| | Editor | Scope | | ||
| | ---------------------- | --------------------------------------------------------------------------------------------------- | | ||
| | `ContainerEditor` | Environment variables, arguments, resource limits, ports | | ||
| | `PodSpecEditor` | Volumes, tolerations, node selectors, service account, security context | | ||
| | `DeploymentSpecEditor` | Replicas, update strategy, label selectors | | ||
| | `ConfigMapDataEditor` | `.data` entries — set, remove, deep-merge YAML patches, raw access | | ||
| | `SecretDataEditor` | `.data` and `.stringData` — set/remove bytes, `SetString`/`RemoveString`, `Raw()`/`RawStringData()` | | ||
| | `ObjectMetaEditor` | Labels and annotations on any Kubernetes object | |
There was a problem hiding this comment.
These rows use || which is not valid GitHub-flavored Markdown table syntax (it creates an empty first column and typically won’t render as intended). Consider switching this to a standard Markdown table using single leading pipes (e.g., | Editor | Scope |) to ensure proper rendering.
| | Capability | Detail | | ||
| | --------------------- | ------------------------------------------------------------------------------------------------ | | ||
| | **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | | ||
| | **Mutation pipeline** | Typed editors for `.data` and `.stringData` entries and object metadata, with a raw escape hatch | | ||
| | **Data extraction** | Reads generated or updated values back from the reconciled Secret after each sync cycle | |
There was a problem hiding this comment.
The capabilities table uses || which does not render as a standard Markdown table in GitHub-flavored Markdown (it generally introduces an unintended empty first column). Update to conventional table syntax (leading | per row) so the documentation renders correctly.
Claude Review Cycle 1 CompleteAddressed: Intentionally ignored:
|
| func (e *SecretDataEditor) Remove(key string) { | ||
| delete(*e.data, key) | ||
| } |
There was a problem hiding this comment.
Remove/RemoveString dereference e.data / e.stringData without guarding against a nil pointer receiver or a zero-value SecretDataEditor (where these fields are nil), which will panic. Since SecretDataEditor is exported, callers can construct it without NewSecretDataEditor; consider making the type resilient by guarding e == nil and e.data == nil / e.stringData == nil (no-op), or make the struct unexported to enforce construction via NewSecretDataEditor.
| func (e *SecretDataEditor) RemoveString(key string) { | ||
| delete(*e.stringData, key) | ||
| } |
There was a problem hiding this comment.
Remove/RemoveString dereference e.data / e.stringData without guarding against a nil pointer receiver or a zero-value SecretDataEditor (where these fields are nil), which will panic. Since SecretDataEditor is exported, callers can construct it without NewSecretDataEditor; consider making the type resilient by guarding e == nil and e.data == nil / e.stringData == nil (no-op), or make the struct unexported to enforce construction via NewSecretDataEditor.
| Mutate: func(m *secret.Mutator) error { | ||
| m.SetStringData("username", "app-user") | ||
| m.SetStringData("password", "default-password") | ||
| return nil | ||
| }, |
There was a problem hiding this comment.
The example bakes credential-like values directly into source (password/token strings). Even for examples, this is a risky pattern to normalize—consider replacing these with clearly non-sensitive placeholders (e.g. \"REPLACE_ME\"), generating random values at runtime for the demo, or reading from environment/flags, and add an explicit comment that these must never be used as-is in real controllers.
| Mutate: func(m *secret.Mutator) error { | ||
| m.SetStringData("otel-auth-token", "trace-secret-token-abc123") | ||
| return nil | ||
| }, |
There was a problem hiding this comment.
The example bakes credential-like values directly into source (password/token strings). Even for examples, this is a risky pattern to normalize—consider replacing these with clearly non-sensitive placeholders (e.g. \"REPLACE_ME\"), generating random values at runtime for the demo, or reading from environment/flags, and add an explicit comment that these must never be used as-is in real controllers.
| Mutate: func(m *secret.Mutator) error { | ||
| m.SetStringData("metrics-auth-token", "metrics-secret-token-xyz789") | ||
| return nil | ||
| }, |
There was a problem hiding this comment.
The example bakes credential-like values directly into source (password/token strings). Even for examples, this is a risky pattern to normalize—consider replacing these with clearly non-sensitive placeholders (e.g. \"REPLACE_ME\"), generating random values at runtime for the demo, or reading from environment/flags, and add an explicit comment that these must never be used as-is in real controllers.
…dentials in examples Add nil-map guards to SecretDataEditor.Remove and RemoveString so they are no-ops when the underlying map is nil, preventing panics on zero-value structs. Replace hard-coded credential-like strings in the example mutations with "REPLACE_ME" placeholders and add a doc comment warning against hard-coding secrets in real controllers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored: <!-- claude-review-cycle --> |
Implements the
secretKubernetes resource primitive following the pattern established by the existingConfigMapandDeploymentprimitives.Summary
secretprimitive package underpkg/primitives/secret/SecretDataEditorto sharedpkg/mutation/editors/(follows existingConfigMapDataEditorpattern)docs/primitives.md) to include the new primitive in the indexChecklist